-
-
Notifications
You must be signed in to change notification settings - Fork 375
require that $schema cannot contain a fragment
#1633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
jdesrosiers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised to see a PR for this. I didn't think there was a consensus on this issue. The discussion just petered out without a clear resolution. I'm providing a review for this result, but I think we need to make sure this is the resolution we want before merging.
My first thought was that we can't do this because older dialect URIs use fragments, but then I realized I don't really know how this would affect older releases. It's a weird chicken and egg problem. I feel like $schema needs to be consistent across releases because it's what's used to determine which release to use. You don't know what version of $schema to use until you read its value.
I think that leaves us with two options: (1) allow (but discourage) an empty fragment, or (2) acknowledge the historic exceptions to the rule.
|
I like option 2 -- it exactly covers what we want to do while accomodating the existence of older drafts. We should still be able to disallow fragments (even empty ones), while still allowing the use of the draft7 and earlier schemas in a new implementation, even with a restriction in the metaschema itself (e.g. with the We would just need to point out the exception for older schemas, so the implementation doesn't reject the use of an empty fragment before considering if an older draft's semantics should apply. |
Co-authored-by: Jason Desrosiers <jdesrosi@gmail.com>
|
By allowing these, are we saying that all historical schemas are valid v1 schemas that just use different dialects, e.g. a draft 6 dialect? Otherwise it seems to me that those schemas would be valid under their own spec versions but not v1, and thus we don't need to state the exception. This is the change I worked up: But it seems odd to explicitly reference older versions like this. |
That definitely doesn't sound right. v1 is a distinct dialect from draft-06.
Maybe, but that's confusing at best. Consider that it's not necessarily just those dialect URIs you listed that could have fragments. An implementation could allow you to define a dialect of any version. I can make a dialect that extends draft-07 and it would be allowed to have a fragment. So, I can't just have an allow list of dialects that are exceptions. The only way is to identify the dialect and then pointlessly validate using the rules of the version it uses. I think what we can say is that implementations MUST NOT allow users to define a dialect of v1 with a URI that isn't absolute and normalized. But, constraining what value I'm not sure where that leaves us. If we want to move forward with this, I wouldn't call out the draft dialects as an exception, but rather include a footnote that says that implementations that support draft versions as well as v1 would need to be more flexible in what they accept to allow for URIs that were allowed in the versions they support. However, I think there's also a "do nothing" option here. In v1, |
| The value of this keyword MUST be an | ||
| [IRI](https://www.rfc-editor.org/info/rfc3987) (containing a scheme) and this | ||
| [absolute IRI](https://www.rfc-editor.org/info/rfc3987) (without a fragment). This | ||
| IRI MUST be normalized. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't just say it needs to be normalized. We need to say what normalization rules apply. Recently we added language about normalization. We probably need to reference that section.
What kind of change does this PR introduce?
Clarification
Issue & Discussion References
$schema#1590Summary
Adds a requirement to
$schemathat its value cannot contain a fragment. This prevents someone from using a non-canonical IRI in this keyword.Does this PR introduce a breaking change?
Technically, yes, but the likelyhood of people using fragments in
$schemais low (probably non-zero, though).